feat: replace static event name with dynamic based on event id #1029

vordimous · 2024-05-15T15:49:48Z

Description

This will use the defined internal eventId to format an observability-friendly in an all-caps snake case.

Fixes #1013

docs PR:

update events page zilla-docs#230

...ain/java/io/aklivity/zilla/runtime/catalog/filesystem/internal/FilesystemEventFormatter.java

.../main/java/io/aklivity/zilla/runtime/exporter/stdout/internal/stream/StdoutEventsStream.java

...p/src/main/java/io/aklivity/zilla/runtime/exporter/otlp/internal/serializer/EventReader.java

.../src/main/java/io/aklivity/zilla/runtime/exporter/stdout/internal/StdoutExporterContext.java

attilakreiner · 2024-05-16T08:37:54Z

LGTM apart from the test failures. It looks like there is test code (EchoWorker and TlsWorker) that also implement the interface EngineContext, hence the compile error.

...g-kafka.spec/src/main/scripts/io/aklivity/zilla/specs/binding/kafka/config/client.event.yaml

specs/model-avro.spec/src/main/scripts/io/aklivity/zilla/specs/model/avro/config/event.yaml

…untime/catalog/filesystem/internal/FilesystemEventFormatter.java Co-authored-by: John Fallows <[email protected]>

…e/exporter/stdout/internal/StdoutExporterContext.java Co-authored-by: John Fallows <[email protected]>

…exporter/otlp/internal/serializer/EventReader.java Co-authored-by: John Fallows <[email protected]>

vordimous · 2024-06-03T16:57:41Z

followup changes needed that will require more extensive changes:

MqttKafkaResetMqttConnectionExFW should include more properties about why a connection was reset, topic names, etc
Remove internal only values from the event messages, ex: identity in the MqttEventContext.onClientConnected event
more precisely handle validation exceptions by not passing all exception messages ex: catch (IOException | AvroRuntimeException ex)
include the exception message for the TLS binding error events.
KafkaEventFormatter needs a map of all ApiKey and ApiVersion codes to render the text version that is relevant to the user. ApiKey enum based on https://kafka.apache.org/protocol.html#protocol_api_keys

specs/binding-mqtt-kafka.spec/src/main/resources/META-INF/zilla/mqtt_kafka.idl

jfallows · 2024-06-03T17:47:49Z

specs/guard-jwt.spec/src/main/scripts/io/aklivity/zilla/specs/guard/jwt/config/event.yaml

@@ -23,7 +23,8 @@ telemetry:
        events:
          - qname: test.net0
            id: guard.jwt.authorization.failed
-            message: AUTHORIZATION_FAILED user
+            name: GUARD_JWT_AUTHORIZATION_FAILED
+            message: No active session found for token identity (user).


This message seems to imply we are looking for matching state in zilla and couldn't find it.

However, I think this event is logged when an attempt to authorize the user (by validating the token) fails, with no implication of state expected to be found at zilla.

Let's reword to capture the underlying behavior without implying expectation to find server-side state, agree?

Updated, please review the new wording with the added reason field.

jfallows · 2024-06-03T17:48:22Z

specs/model-avro.spec/src/main/scripts/io/aklivity/zilla/specs/model/avro/config/event.yaml

@@ -23,7 +23,8 @@ telemetry:
        events:
          - qname: test.net0
            id: model.avro.validation.failed
-            message: VALIDATION_FAILED java.io.EOFException
+            name: MODEL_AVRO_VALIDATION_FAILED
+            message: A message payload failed validation. java.io.EOFException.


Please remove exception class name from message.

"java.io.EOFException" is one of the exception messages returned from the AvroRuntimeException class. The error string passed in the event is set from various exception messages.

I think this is just a bad example of what kind of errors might come through and I don't know how the tests makes this specific exception happen or how to change it to something else.

origional comment here: #1029 (comment)

I also added fixing this as a note in some the follow on modifications that are needed: #1029 (comment)

.../guard-jwt/src/main/java/io/aklivity/zilla/runtime/guard/jwt/internal/JwtEventFormatter.java

...rc/main/java/io/aklivity/zilla/runtime/catalog/karapace/internal/KarapaceEventFormatter.java

...ding-tls/src/main/java/io/aklivity/zilla/runtime/binding/tls/internal/TlsEventFormatter.java

…nding/tls/internal/TlsEventFormatter.java Co-authored-by: John Fallows <[email protected]>

…ent-name

vordimous requested review from jfallows and attilakreiner May 15, 2024 15:49

vordimous linked an issue May 15, 2024 that may be closed by this pull request

Use full Event ID and the event name #1013

Closed

jfallows requested changes May 15, 2024

View reviewed changes

vordimous force-pushed the feature/1013-use-full-event-id-and-the-event-name branch from 81138a5 to fd76938 Compare May 15, 2024 18:38

vordimous requested a review from jfallows May 15, 2024 18:47

jfallows requested changes May 16, 2024

View reviewed changes

...g-kafka.spec/src/main/scripts/io/aklivity/zilla/specs/binding/kafka/config/client.event.yaml Show resolved Hide resolved

specs/model-avro.spec/src/main/scripts/io/aklivity/zilla/specs/model/avro/config/event.yaml Outdated Show resolved Hide resolved

vordimous force-pushed the feature/1013-use-full-event-id-and-the-event-name branch from 218b074 to 935a84e Compare May 20, 2024 14:41

vordimous force-pushed the feature/1013-use-full-event-id-and-the-event-name branch 2 times, most recently from 720d608 to 016ca37 Compare May 30, 2024 14:29

vordimous and others added 12 commits June 3, 2024 08:34

feat: replace static event name with dynamic based on event id

13d20ae

update event.name test value

8c44723

Update incubator/catalog-filesystem/src/main/java/io/aklivity/zilla/r…

db6c249

…untime/catalog/filesystem/internal/FilesystemEventFormatter.java Co-authored-by: John Fallows <[email protected]>

Update runtime/exporter-stdout/src/main/java/io/aklivity/zilla/runtim…

a9b16cd

…e/exporter/stdout/internal/StdoutExporterContext.java Co-authored-by: John Fallows <[email protected]>

Update runtime/exporter-otlp/src/main/java/io/aklivity/zilla/runtime/…

db918eb

…exporter/otlp/internal/serializer/EventReader.java Co-authored-by: John Fallows <[email protected]>

Build event name in Engine context

5b3b00d

fix build errors

35f3254

update tests

5608829

fix http binding event test

34deb18

move event names out of format

981fed4

correct code to pass tests

7bff9fe

refactor the compact session topic event

1ba8a8e

vordimous force-pushed the feature/1013-use-full-event-id-and-the-event-name branch from 405c718 to 1ba8a8e Compare June 3, 2024 12:34

vordimous added 6 commits June 3, 2024 10:26

move the static reason into the event formatter

30430b8

update model validation event messages

f3ccc93

update jwt guard event message

c45effe

update catalog filesystem event message

74af1f5

Add generic tls error message

d11f441

update tcp event message

0d122ac

vordimous added 3 commits June 3, 2024 11:46

add context to mqtt client connected message

7bdd8e3

update kafka and http message formatting

a23579b

update apicurio artifact fetch messages

4086ca4

vordimous force-pushed the feature/1013-use-full-event-id-and-the-event-name branch from 7a3884b to 4086ca4 Compare June 3, 2024 16:55

vordimous added 2 commits June 3, 2024 13:29

update karapace schema registry message formatting

369e109

fix test

6cd31b3

vordimous marked this pull request as ready for review June 3, 2024 17:33

vordimous requested a review from jfallows June 3, 2024 17:33

jfallows requested changes Jun 3, 2024

View reviewed changes

vordimous and others added 4 commits June 3, 2024 14:24

Update runtime/binding-tls/src/main/java/io/aklivity/zilla/runtime/bi…

656b19a

…nding/tls/internal/TlsEventFormatter.java Co-authored-by: John Fallows <[email protected]>

Improve jwt message with added reason

187bd0e

rename

140f628

improve mqtt session topic compated event message

6377a8a

vordimous force-pushed the feature/1013-use-full-event-id-and-the-event-name branch from 1216049 to 6377a8a Compare June 3, 2024 19:15

update code coverage for guard-jwt project to 0.97

0b38f7a

vordimous requested a review from jfallows June 3, 2024 20:24

jfallows previously approved these changes Jun 4, 2024

View reviewed changes

vordimous mentioned this pull request Jun 4, 2024

update events page aklivity/zilla-docs#230

Merged

Merge branch 'develop' into feature/1013-use-full-event-id-and-the-ev…

5d6e603

…ent-name

vordimous dismissed jfallows’s stale review via 5d6e603 June 5, 2024 12:23

jfallows approved these changes Jun 5, 2024

View reviewed changes

jfallows merged commit 354a4e1 into develop Jun 5, 2024
8 checks passed

vordimous deleted the feature/1013-use-full-event-id-and-the-event-name branch June 11, 2024 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: replace static event name with dynamic based on event id #1029

feat: replace static event name with dynamic based on event id #1029

vordimous commented May 15, 2024 •

edited

Loading

attilakreiner commented May 16, 2024

vordimous commented Jun 3, 2024 •

edited

Loading

jfallows Jun 3, 2024

vordimous Jun 3, 2024 •

edited

Loading

jfallows Jun 3, 2024

vordimous Jun 3, 2024

vordimous Jun 3, 2024 •

edited

Loading

feat: replace static event name with dynamic based on event id #1029

feat: replace static event name with dynamic based on event id #1029

Conversation

vordimous commented May 15, 2024 • edited Loading

Description

attilakreiner commented May 16, 2024

vordimous commented Jun 3, 2024 • edited Loading

jfallows Jun 3, 2024

Choose a reason for hiding this comment

vordimous Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

jfallows Jun 3, 2024

Choose a reason for hiding this comment

vordimous Jun 3, 2024

Choose a reason for hiding this comment

vordimous Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

vordimous commented May 15, 2024 •

edited

Loading

vordimous commented Jun 3, 2024 •

edited

Loading

vordimous Jun 3, 2024 •

edited

Loading

vordimous Jun 3, 2024 •

edited

Loading